fix(kotlin-sdk): reconcile the TXO store against the engine and repair restored address pools - #4439

🔴 Blocking: Do not persist unconfirmed inputs as spent

The exact pinned key-wallet revision inserts every input of a recorded transaction into spent_outpoints, regardless of whether its context is mempool or in-block. Treating membership in that set as a confirmed spend contradicts this handler's established rule at lines 928-934 and 3073-3080: mempool-linked inputs remain unspent in Room so they can be restored and reclassified after restart. A reconciliation while a payment is unconfirmed therefore makes the coin durably spent; if the transaction is later abandoned and its release update is lost or interrupted, the deliberate never-unmark policy prevents every later reconciliation from recovering it. Export spender context/finality with each outpoint, or only flip rows for spends proven confirmed by another authoritative source.

_{source: ['codex']}

Resolved in 2195b18 — Do not persist unconfirmed inputs as spent no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

Resolved in 2195b18 and still true after the rebase: the reverse pass only classifies and logs, nothing is flipped.

🔴 Blocking: Only account for TXOs that were actually inserted

upsertUtxoRow returns early when the parent transaction is still marked isGloballySwept, but it returns Unit, so this caller cannot distinguish that refusal from a successful insert. The reconciler then increments inserted, adds to insertedDuffs, and may update netAmount even though the TXO remains absent. A missed reinstatement record can leave exactly this stale tombstone while the engine authoritatively holds the output; every periodic pass then repeats the false heal and can repeatedly add the same amount. Make the helper report whether it materialized the row and perform all counters and amount repair only after a successful insert, or explicitly reconcile the stale swept state first.

_{source: ['codex']}

Resolved in 2195b18 — Only account for TXOs that were actually inserted no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

The refusal path no longer exists on v4.2-dev (no isGloballySwept flag). upsertUtxoRow is v4.2-dev's own insert discipline extracted into one helper and always writes, so the counter and the skippedSwept report field are gone.

🔴 Blocking: Do not infer netAmount from TXO presence

A missing txos projection does not prove that the independently persisted transaction record omitted the output from netAmount. For example, a corrective transaction callback can already store the engine's recomputed net amount while delivery of the corresponding UTXO projection is omitted, or a TXO can disappear later without changing its parent transaction. In either case this unconditional delta overstates the transaction, and the newly inserted row makes the corruption permanent because later passes become no-ops. The inventory does not include an authoritative expected net amount, so repair must compare against one or recompute the amount from authoritative ownership data rather than infer it from row absence.

_{source: ['codex']}

Resolved in 2195b18 — Do not infer netAmount from TXO presence no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

Done in 2195b18: netAmount is never mutated; a healed TXO with a pre-existing record is only reported as a netAmountSuspect.

🔴 Blocking: Repair DashPay pools with their concrete account xpub

Both DashPay variants reach this resolver, but the exact pinned implementation of Wallet::key_source_for_account_type explicitly returns NoKeySource for DashpayReceivingFunds and DashpayExternalAccount. The guard at line 4131 therefore always skips their hole repair. This is especially harmful for DashpayReceivingFunds: it is wallet-owned and funds-bearing, and its concrete account already carries the xpub needed to reconstruct missing addresses. Resolve the full AccountType through wallet.accounts.account_of_type(account_type) and use that account's xpub as KeySource::Public, retaining the existing helper as the fallback for special account types such as the BLS provider account. Add a restore test with a sparse DashPay receiving pool and a real signing wallet; the current test passes None and cannot exercise this path.

_{source: ['codex']}

Resolved in 2195b18 — Repair DashPay pools with their concrete account xpub no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

Done in 2195b18 (key source resolved from the signing wallet). DashPay contact pools are re-derived by contact sync at runtime; that skip is now logged at debug.

Fixed in 80d61e8 as you describe: the resolver consults accounts.account_of_type(account_type) first and uses that account's xpub as KeySource::Public, keeping key_source_for_account_type as the fallback for the types account_of_type does not return (the BLS provider operator and Ed25519 platform-node accounts). New test dashpay_receiving_pool_holes_are_repaired_from_the_account_xpub restores a sparse DashPay receiving pool with a real signing wallet; without the fix it fails with 30 holes left at indices 20–49, with it every index up to the watermark is derivable. Leaving the thread for you to resolve.

🟡 Suggestion: Resolve contact ownership through coreAddressId

The exclusion only checks row.accountId, but production changeset writes leave that field null and route TXO ownership through coreAddressId -> core_addresses.accountId, as documented by buildUtxoRestoreData at lines 3065-3069. The regression test manually fills accountId, so it does not represent production contact rows. Resolve the effective account through coreAddressId when the direct FK is null in both reverse-pass loops; otherwise normal DIP-15 contact outputs are repeatedly reported as engine-unknown and defeat the intended regression-tripwire signal.

_{source: ['codex']}

Resolved in 2195b18 — Resolve contact ownership through coreAddressId no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

Done in 2195b18: rowIsForeign resolves both accountId and coreAddressId → core_addresses.accountId, and the insert pass uses the inventory's account tag first.

🔴 Blocking: Preserve exported account ownership on healed TXOs

The native inventory identifies the account that owns each UTXO, but this insertion ignores those tags. upsertUtxoRow preserves an existing accountId and sets coreAddressId only when the corresponding core_addresses row already exists. If persistence lost both the TXO and its address row—the two divergence classes this PR is intended to repair—the reconciled row is inserted with neither ownership link. On the next launch, buildUtxoRestoreData can resolve ownership only through txo.accountId or the address relationship, so it skips the healed output and recreates the visible fund loss. Emit the complete account tuple already present in AccountBalanceEntryFFI (typeTag, standardTag, index, registration index, key class, and DashPay identity IDs), resolve the Room account during reconciliation, and persist its ID on the healed TXO even when the address projection is absent.

_{source: ['codex']}

Done in 4c6c6ec and kept: every row carries the account tuple, the reconcile resolves the Room account and stamps resolvedAccountId on the healed row (healedUnowned counts the unresolvable ones).

🟡 Suggestion: Interpolate values in the healed-TXO diagnostic

Each dollar sign is escaped with ${'$'}, so the diagnostic emits literal placeholders such as ${txid.toHex()}:$vout, $amount, and ${priorTx.netAmount}. That removes the outpoint and values needed to investigate the nonzero reconciliation heal this warning is intended to identify.

_{source: ['codex']}

Fixed: the diagnostic now interpolates txid:vout, amount and storedNet.

The first-page prefetch is correct only under an invariant nothing enforces.

firstPage is fetched eagerly and smuggled back through a stateful closure keyed on cursor == null, purely to preserve the "return null on dead transport" signature. The comment states the load-bearing assumption — "The handler asks for the null cursor exactly once" — but reconcileTxos is a public method on another class with no such contract in its own docs.

Any future change there (a retry after a transport failure, a second sweep, restarting the insert pass) would silently replay a stale page and re-walk rows already applied, with no test covering it.

Cheaper and self-enforcing: pass ::page straight through and have reconcileTxos return null itself when the first page comes back null — the null-cursor call is already distinguishable inside the loop.

🤖 Posted autonomously by Claude on behalf of pasta.

Gone (c5cbc44): reconcileTxoStore passes the page lambda straight through, and reconcileTxos returns null itself when the first page is null (test reconcileReturnsNoReportWhenTheFirstPageIsUnavailable).

reconcileTxos is ~400 lines holding two unrelated passes and 17 mutable counters in one scope.

The insert pass and the classification pass share exactly one piece of state — foreignAccountIds — yet every counter is live across both: skippedImmature/healedUnowned/netAmountSuspects/skippedSwept belong only to the first, wouldFlipSpent/stuckSpent/wouldRemove only to the second. The insert loop nests five deep (while → withLock → withTransaction → for → the skip chain), and both passes declare local suspend funs inside it.

Any edit to either half currently requires reading the whole body to know which counters are in play. Two private functions — healMissingTxos(...) and classifyStoreRows(...), each returning its own half of the report — would leave reconcileTxos as: resolve foreignAccountIds, call both, merge, log.

🤖 Posted autonomously by Claude on behalf of pasta.

Split (c5cbc44): healMissingTxos (+ healEngineRow per row) and classifyStoreRows each return their half (HealPass / ClassifyPass); reconcileTxos resolves the foreign account set, calls both, merges and logs.

Done in c5cbc44 and carried forward: reconcileTxos is now ~80 lines that resolve the shared exclusion set, call healMissingTxos and classifyStoreRows, merge the two halves and log. Leaving the thread for you to resolve.

[correctness — confirmed mechanism] The null-transport contract is unreachable: mapNativeErrors rethrows, it never returns null.

mapNativeErrors (DashSdkError.kt:610) catches DashSDKException and throws DashSdkError.fromNative(e) — and the JNI side (guard + take_pwffi_error) throws an SDK exception whenever the native call fails. So page(...) propagates an exception on transport failure rather than returning null:

• val firstPage = page(null, pageSize) ?: return null — the documented "returns null when the first page fails" path never fires; callers get an exception instead.
• In reconcileTxos, if (pageJson == null) { transportFailures++; break } (and the classify equivalent) never runs for real JNI failures — a mid-sweep transport error aborts the whole pass by exception, discarding the report the truncate-and-report design promises. maybeReconcileTxoStores swallows it via runCatching, so the partial-sweep telemetry is silently lost.

Either wrap the two lambdas in runCatching { ... }.getOrNull() so the handler's null-handling actually engages, or update the contract docs to exception semantics.

🤖 Posted autonomously by Claude on behalf of pasta.

Fixed (c5cbc44): both transport lambdas wrap mapNativeErrors in runCatching { … }.getOrNull() and log the fault, so a mid-sweep native failure yields the partial report the truncate-and-report design promises.

[correctness — plausible] tipHeight comes solely from progress.filters?.currentHeight; when the filters sub-phase is absent (has_filters == false maps to filters = null in SpvSyncProgressData.fromNative) or reports 0, maybeReconcileTxoStores returns before scheduling anything — silently, on every tick.

If dash-spv ever reports SYNCED without a filters sub-progress (filter sync disabled, or the phase dropped after completion), the entire heal this PR exists to deliver never runs, with no log line to say so. Consider falling back to headers?.currentHeight (or the wallet's synced height accessor) and logging when the sweep is skipped for want of a tip height.

🤖 Posted autonomously by Claude on behalf of pasta.

Fixed (c5cbc44): falls back to headers.currentHeight and logs when neither height is available instead of returning silently.

[conventions — packages/kotlin-sdk/CLAUDE.md] walletManagerUtxosPageJson violates the stated rule: "No JNI functions that stitch together existing Rust calls — add the composite to Rust instead."

This export stitches platform_wallet_manager_get_account_balances + per-account platform_wallet_account_utxos_page, and layers on its own cross-account ordering (account_sort_key), an ad-hoc <accountKeyHex>:<txidHex>:<vout> cursor format, hand-rolled hex and JSON encoders, and dashcore address encoding — all in the JNI shim, where the account-key layout can drift from AccountBalanceEntryFFI with no compile error. A wallet-wide paged accessor beside account_utxos_page_blocking in rs-platform-wallet would own the invariant once, and the Swift host would get it for free.

🤖 Posted autonomously by Claude on behalf of pasta.

Fixed (ca3c739): the composite lives in rs-platform-wallet (wallet_utxos_page_blocking) and the FFI (platform_wallet_wallet_utxos_page); the JNI is a serializer over one call, and the Swift host gets the same walk.

The composite is gone. The export is a serde shim over one call, upstream's platform_wallet_wallet_utxos_page from #4638; account ordering, cursor semantics, the page bound and the address rendering all live in platform-wallet now, and the dashcore dependency is out of the JNI crate. Leaving the thread for you to resolve. #4439 (comment)